src/test/java/org/example/visacasemanagementsystem/file/FileServiceIntegrationTest.java (3)

42-42: Typo in test method name: ShouldReturnSKey → ShouldReturnS3Key.

Minor naming polish for readability in test reports.

✏️ Proposed rename

-    void uploadFile_ShouldReturnSKey_WhenUploadIsSuccessful() throws IOException {
+    void uploadFile_ShouldReturnS3Key_WhenUploadIsSuccessful() throws IOException {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/file/FileServiceIntegrationTest.java`
at line 42, Rename the test method
uploadFile_ShouldReturnSKey_WhenUploadIsSuccessful() to
uploadFile_ShouldReturnS3Key_WhenUploadIsSuccessful() to fix the typo; update
the method declaration and any references to that method (e.g., in test suites
or IDE run configurations) so the test name reads "S3Key" for clarity in test
reports.

---

9-17: Redundant/overlapping imports.

Line 15 uses a wildcard software.amazon.awssdk.services.s3.model.* which already covers PutObjectRequest and PutObjectResponse explicitly imported on lines 11–12. Either drop the wildcard and keep explicit imports (preferred) or remove the now-redundant explicit ones. Also, the two org.assertj.core.api.Assertions static imports on lines 14 and 23 could be consolidated into a single block.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/file/FileServiceIntegrationTest.java`
around lines 9 - 17, The test file has overlapping imports: remove the broad
wildcard import software.amazon.awssdk.services.s3.model.* and keep the explicit
imports like PutObjectRequest and PutObjectResponse (and other needed model
types) so imports are explicit; also consolidate duplicate static imports of
org.assertj.core.api.Assertions.assertThat into a single static import statement
to avoid redundancy and improve clarity—update the import block to only include
S3Client, S3Presigner, the explicit model classes you use (e.g.,
PutObjectRequest, PutObjectResponse, GetObjectRequest, etc.), and a single
static import for Assertions.assertThat.

---

142-154: Add a test for error handling in deleteFile.

The current test covers the happy path. Since deleteFile does not explicitly handle exceptions, adding a test that stubs s3Client.deleteObject(...) to throw an exception would document the error-handling contract and ensure the method propagates exceptions as expected.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/file/FileServiceIntegrationTest.java`
around lines 142 - 154, Add a test in FileServiceIntegrationTest that verifies
deleteFile propagates exceptions by stubbing s3Client.deleteObject(...) to throw
(use Mockito doThrow/when) and asserting that calling
fileService.deleteFile(s3Key) results in an exception (use assertThrows),
referencing the deleteFile method, s3Client.deleteObject and DeleteObjectRequest
to locate the call and ensure the test verifies propagation rather than
swallowing the error.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/test/java/org/example/visacasemanagementsystem/file/FileServiceIntegrationTest.java (3)

77-79: Assert validation failures do not attempt an S3 upload.

The exception assertions are good, but these tests would also pass if validation happened after an attempted putObject side effect. Add a negative verification for the upload call.

Suggested negative verifications

         assertThatThrownBy(() -> fileService.uploadFile(largeFile))
                 .isInstanceOf(IOException.class)
                 .hasMessageContaining("File exceeds maximum allowed size of 10 MB");
+        verify(s3Client, never()).putObject(any(PutObjectRequest.class), any(RequestBody.class));

         assertThatThrownBy(() -> fileService.uploadFile(mockFile))
         .isInstanceOf(IOException.class)
                 .hasMessageContaining("Unsupported document type: " + mockFile.getContentType());
+        verify(s3Client, never()).putObject(any(PutObjectRequest.class), any(RequestBody.class));

Also applies to: 115-117

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/file/FileServiceIntegrationTest.java`
around lines 77 - 79, The tests currently assert that
fileService.uploadFile(largeFile) throws an IOException but don’t assert that no
S3 upload was attempted; after each assertThatThrownBy add a negative
verification that the S3 client’s upload method was not invoked (e.g.
verify(s3Client, never()).putObject(...)) to ensure validation fails before any
call to the S3 upload; apply the same change to the second failing test around
lines 115-117 as well so both tests verify no putObject/upload call was made.

---

132-139: Avoid any() for the presign request.

This test currently only proves that url() is returned from the mocked presigner. It would still pass if FileService built the presign request with the wrong key, bucket, response disposition, or duration.

Suggested request matcher

-       when(s3Presigner.presignGetObject(any(GetObjectPresignRequest.class)))
+       when(s3Presigner.presignGetObject(argThat((GetObjectPresignRequest req) ->
+               req.signatureDuration().equals(java.time.Duration.ofMinutes(10))
+                       && "test-bucket".equals(req.getObjectRequest().bucket())
+                       && fileName.equals(req.getObjectRequest().key())
+                       && ("attachment; filename=\"" + fileName + "\"")
+                               .equals(req.getObjectRequest().responseContentDisposition()))))
                .thenReturn(mockPresignedRequest);

Please verify the exact accessor names against the AWS SDK v2 version used by the project if applying this matcher.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/file/FileServiceIntegrationTest.java`
around lines 132 - 139, The test uses any(GetObjectPresignRequest.class) when
stubbing s3Presigner.presignGetObject, so it doesn’t verify that
FileService.getPresignedDownloadUrl builds the presign request with the correct
bucket, key, content-disposition and duration; replace the any() stub with a
Mockito argument matcher (e.g., argThat) that inspects the
GetObjectPresignRequest passed into s3Presigner.presignGetObject and asserts the
expected values (check request.bucket(), request.key(),
request.getObjectRequest().responseContentDisposition() or equivalent accessor
for content-disposition, and the request signature/duration accessor used by the
AWS SDK v2) before returning mockPresignedRequest so the test fails if
FileService constructs the wrong request.

---

51-65: Verify the uploaded key and content type, not just the bucket.

This test would still pass if uploadFile uploaded a different key than the one it returns, or omitted the request content type. Tighten the matcher to cover the S3 request contract.

Suggested assertion hardening

         verify(s3Client).putObject(
-                argThat((PutObjectRequest req) -> req.bucket().equals("test-bucket")),
+                argThat((PutObjectRequest req) ->
+                        "test-bucket".equals(req.bucket())
+                                && resultKey.equals(req.key())
+                                && "application/pdf".equals(req.contentType())),
                 any(RequestBody.class)
         );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/file/FileServiceIntegrationTest.java`
around lines 51 - 65, The test currently only verifies S3 bucket and returned
filename; tighten it by asserting the actual PutObjectRequest contains the
expected object key and content type so uploadFile cannot return a different key
than it uploaded or omit content type. After calling
fileService.uploadFile(mockFile), capture the expected key (the returned
resultKey) and update the verify on s3Client.putObject to argThat a
PutObjectRequest whose key() equals that expected key and whose contentType()
(or contentType header/metadata) equals "application/pdf" (or the MIME your
service sets), and keep the RequestBody matcher as any(RequestBody.class). Also
keep the existing assertions on resultKey format to ensure both the return value
and the actual S3 request are validated.

src/test/java/org/example/visacasemanagementsystem/file/FileServiceIntegrationTest.java (1)

11-17: Redundant wildcard import overlaps with explicit imports.

Line 15 imports software.amazon.awssdk.services.s3.model.*, which already covers PutObjectRequest (line 11) and PutObjectResponse (line 12). Collapse to a single form to avoid noise.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/file/FileServiceIntegrationTest.java`
around lines 11 - 17, The import block contains a redundant wildcard import
`software.amazon.awssdk.services.s3.model.*` which overlaps explicit imports
like PutObjectRequest and PutObjectResponse; remove the wildcard and keep the
explicit imports (PutObjectRequest, PutObjectResponse, S3Presigner,
GetObjectPresignRequest, PresignedGetObjectRequest and any other used symbols
such as assertThat) so only the exact required types are imported in
FileServiceIntegrationTest.